Trim UnixFileSystemTypes to the values .NET runtime cares about. #122964

tmds · 2026-01-07T10:14:36Z

@am11 @stephentoub @jkotas @adamsitnik ptal.

src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs

… can lock.

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

src/native/libs/System.Native/pal_io.c

tmds · 2026-01-08T09:49:44Z

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

-                default:
-                    return true; // in all other situations it should be OK
-            }
+            return Interop.Sys.FileSystemSupportsLocking(this);


The behavior here is changing. Previously, if we didn't recognize the type as one of the values in UnixFileSystemTypes then no locking was performed. Now, we'll do locking unless the type is nfs/smb/cifs.

I assume we're ok to do this as return true also happens in earlier checks.

This does mean that we rely on the string names in PAL to be correct for identifying nfs/smb/cifs.

This does mean that we rely on the string names in PAL to be correct for identifying nfs/smb/cifs.

Isn't it the same as main branch? f_type on linux (comparison with magic integers) and f_fstypename / f_basetype on other Unix (string comparison with strcmp)

Suppose we have the names wrong.

On the main branch, MapFileSystemNameToEnum returns zero, then TryGetFileSystemType return false and then CanLockTheFile will return false.

With the new implementation, FileSystemNameSupportsLocking returns 1 and then FileSystemSupportsLocking will return true.

The comment above says "// LOCK_SH is not OK when writing to NFS, CIFS or SMB". Why is it not OK? What's the failure more?

I am trying to better understand the downside of enabling locking by default for all file systems except the few we explicitly check for.

Locking is enabled by default to match Windows behavior.
(For example: you get an exception when the SDK tries to write the same file simultaneously.)

The code is filtering for a combination that doesn't work well.
The combination seems to be: LOCK_SH + FileAccess.Write + nfs/smb/cifs.
And the referenced issues mention silently discarded writes and throwing on dispose (#44546, #53182).

It sounds like that this is a workaround for a buggy implementation of locks in some file systems. I think it is fine to assume that unknown file systems have a correct lock implementation.

cc @adamsitnik

am11

Looks great. Thanks! :)

tmds · 2026-01-09T14:09:37Z

Thanks for the suggestions/review, @am11!

jkotas · 2026-01-11T16:05:55Z

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

-                default:
-                    return true; // in all other situations it should be OK
-            }
+            return Interop.Sys.FileSystemSupportsLocking(this);


Suggested change

return Interop.Sys.FileSystemSupportsLocking(this);

// LOCK_SH does not work well for write access in some file systems. For example, writes are dropped silently.

// We try to detect the problematic cases and disable use of LOCK_SH for write access for them.

// See https://github.com/dotnet/runtime/issues/44546 and https://github.com/dotnet/runtime/issues/53182

return Interop.Sys.FileSystemSupportsLocking(this);

jkotas · 2026-01-11T16:16:44Z

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs

-                default:
-                    return true; // in all other situations it should be OK
-            }
+            return Interop.Sys.FileSystemSupportsLocking(this);


Should this method have a better name to make it clear that this is just about the SH_LOCK + write combination?

Alternatively, this method can take the lock type and bool to indicate write access to make the contract easier to explain.

Alternatively, this method can take the lock type and bool to indicate write access to make the contract easier to explain.

I've implemented this option.

… comment.

Trim UnixFileSystemTypes to the values .NET runtime cares about.

136f4f1

github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 7, 2026

dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jan 7, 2026

am11 reviewed Jan 7, 2026

View reviewed changes

src/libraries/Common/src/Interop/Unix/System.Native/Interop.UnixFileSystemTypes.cs Outdated Show resolved Hide resolved

am11 added area-System.IO and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 7, 2026

tmds added 2 commits January 7, 2026 18:13

Replace SystemNative_GetFileSystemType by a method that returns if we…

1da80aa

… can lock.

Fix CI build failure.

da21361

am11 reviewed Jan 7, 2026

View reviewed changes

src/libraries/System.Private.CoreLib/src/Microsoft/Win32/SafeHandles/SafeFileHandle.Unix.cs Outdated Show resolved Hide resolved

am11 reviewed Jan 7, 2026

View reviewed changes

src/native/libs/System.Native/pal_io.c Outdated Show resolved Hide resolved

tmds added 3 commits January 7, 2026 19:20

Simplify CanLockTheFile.

9809b7c

Handle f_type as an unsigned int.

d05c899

Fix CI build errors.

2554fe2

build-analysis bot mentioned this pull request Jan 7, 2026

AF: *(_UNCHECKED_OBJECTREF *)handle == NULL (HndCreateHandle called by getJitHandleForObject) #117138

Open

tmds commented Jan 8, 2026

View reviewed changes

am11 approved these changes Jan 8, 2026

View reviewed changes

jkotas requested a review from adamsitnik January 11, 2026 15:50

jkotas reviewed Jan 11, 2026

View reviewed changes

Move the lockOperation and fileAccess check to pal. Add a descriptive…

1b6f3ef

… comment.

jkotas approved these changes Jan 12, 2026

View reviewed changes

This was referenced Jan 12, 2026

"We stopped hearing from agent Azure Pipelines 32. Verify the agent machine is running and has a healthy network connection" dotnet/dnceng#1886

Open

Unable to pull image from mcr.microsoft.com #117164

Open

-            return Interop.Sys.FileSystemSupportsLocking(this);
+            // LOCK_SH does not work well for write access in some file systems. For example, writes are dropped silently.
+            // We try to detect the problematic cases and disable use of  LOCK_SH for write access for them.
+            // See https://github.com/dotnet/runtime/issues/44546 and https://github.com/dotnet/runtime/issues/53182
+            return Interop.Sys.FileSystemSupportsLocking(this);

Trim UnixFileSystemTypes to the values .NET runtime cares about. #122964

Are you sure you want to change the base?

Trim UnixFileSystemTypes to the values .NET runtime cares about. #122964

Uh oh!

Conversation

tmds commented Jan 7, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jkotas Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

am11 left a comment

Choose a reason for hiding this comment

Uh oh!

tmds commented Jan 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jkotas Jan 9, 2026 •

edited

Loading